Skip to content

Refactor bus wire API to use driver handles#64

Merged
xross merged 3 commits into
xmos:developfrom
xross:feat/bus_api
Jun 22, 2026
Merged

Refactor bus wire API to use driver handles#64
xross merged 3 commits into
xmos:developfrom
xross:feat/bus_api

Conversation

@xross

@xross xross commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Refactor BusWire driver API to use OO driver handles

There was a desire to make the resolved-wire API more object-oriented and easier to read at call sites. Instead of passing a driver identifier into every wire operation, drivers are now registered against wires through a Bus, which binds each driver to per-wire handles.
This adds some setup and indirection internally: the bus topology is declared up front, and each BusDriver gets real per-wire attributes via setattr() so call sites can use controller.sda directly. The underlying wire model still tracks driver state internally, but callers no longer pass string/enum driver names for each operation.

Old:

self._sda.release(I3CBusDriver.CONTROLLER)

New:

self._controller.sda.release()

The refactor removes the old string-based driver API, validates wire/driver names during Bus construction, and keeps the existing resolution/clash-detection behavior unchanged.

There are tradeoffs. The new API makes the operation site nicer and more self-describing, but it adds more objects, upfront bus binding, and some internal indirection. To support the controller.sda style without getattr, the implementation uses setattr() to expose bound wire handles as real attributes on each BusDriver. The underlying wire model still tracks per-driver state internally.

One alternative would be index-based access, for example controller[SDA].release() or controller.wire(SDA).release(). That would avoid dynamic attributes and the Python-identifier restriction on wire names, but it is less readable for protocol wires like scl/sda.

So this PR is partly a design tradeoff: if the extra setup and indirection are not worth the cleaner call sites, we may decide to keep the older string/enum-based API instead as it is flatter and easier to understand internally.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the Pyxsim.bus resolved-wire model to use object-oriented “driver handles” bound to wires via a Bus, enabling call sites like controller.sda.release() instead of passing driver identifiers into every operation.

Changes:

  • Introduces Bus, BusDriver, and BusWireDriver to bind drivers to per-wire handles exposed as real attributes (setattr()).
  • Removes the old string/enum-based BusWire public driver-operation API in favor of internal _drive/_release/_set_driver.
  • Updates and expands unit tests to cover the new OO binding, uniqueness validation, and attribute exposure behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/python/Pyxsim/bus.py Adds OO driver-handle API (Bus, BusDriver, BusWireDriver) and refactors BusWire driver operations into internal methods.
tests/test_bus_line_model.py Migrates tests to OO call sites and adds new tests for binding/validation and per-driver behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/python/Pyxsim/bus.py
Comment thread lib/python/Pyxsim/bus.py Outdated
Comment thread lib/python/Pyxsim/bus.py

@humphrey-xmos humphrey-xmos left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, yes it does create more objects and more setup code, but hopefully its easier to extend.

controller.sda.drive(value)


# New OO API-specific tests

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also support multiple targets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, multiple "Drivers", where a driver can be labels anything you like.

xross added 2 commits June 22, 2026 11:53
Validate wire names before attribute binding so invalid API input reports clear ValueErrors, ensure repeated binds hit the intended duplicate guard, and make duplicate-name diagnostics deterministic.
@xross xross merged commit 0481d61 into xmos:develop Jun 22, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants